Skip to content

Fixes for forkserver/spawn serialization and fix for LMDB upgrade issues#148

Open
christinaflo wants to merge 5 commits intomainfrom
feature/multiprocessing-lmdb-refactor
Open

Fixes for forkserver/spawn serialization and fix for LMDB upgrade issues#148
christinaflo wants to merge 5 commits intomainfrom
feature/multiprocessing-lmdb-refactor

Conversation

@christinaflo
Copy link
Copy Markdown
Collaborator

Summary
LMDB refactor to allow for forkserver/spawn serialization and resolve issues that required pinning to LMDB==1.6.2 for training.

Changes

  1. Exposes more dataloader options like persistent workers, prefetch factor, multiprocessing context
  2. Lazy load CCD obj to prevent issues with serialization
  3. Modify LMDB handling to allow for serialization. Close parent db connection prior to forking.

Related Issues
PR #143 defaults to forkserver. This PR adds additional fixes needed for training with forkserver.

…ng methods. LMDB refactor to allow for forkserver/spawn serialization + resolve issues that required pinning lmdb for training.
…ure connection is closed when dataset init finishes prior to forking
@christinaflo christinaflo requested review from jandom and jnwei March 26, 2026 02:32
@christinaflo christinaflo self-assigned this Mar 26, 2026
@sdvillal
Copy link
Copy Markdown

sdvillal commented Apr 8, 2026

Minor comment: the DB preloading logic might be better handled by using vmtouch, which is available in conda‑forge and often present on HPC systems, falling back to reading the whole DB file only when vmtouch is not available. Conceptually, this would only need to be done once per node, since the page cache is shared across processes.

If using vmtouch, one could optionally consider daemon mode and page locking to reduce the risk of eager eviction under memory pressure, although this may or may not be desirable depending on system limits and sharing policies (I assume we typically reserve full nodes?). Overall, I suspect the usefulness of this depends strongly on DB size vs memory size, access locality, and competition for memory.

I just found this which could be vendored. I never used it.

@jandom
Copy link
Copy Markdown
Collaborator

jandom commented Apr 8, 2026

Taking a look at this now @christinaflo, because this came up in a number of different PRs, prominently these two

@jandom jandom self-assigned this Apr 8, 2026
Copy link
Copy Markdown
Collaborator

@jandom jandom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need this any more with the changes in #143 – the two small reads from LMDB will be immediately released now (via context manager) and the persistent lmdb env is staying for the entire duration of the data loader (hopefully!)

but I'm 100% confident – training tends to reveal more problems than running unit tests

Do you have any repro/test that I could run to confirm that my claim is accurate?

@christinaflo
Copy link
Copy Markdown
Collaborator Author

These changes address a different issue than the one addressed in #143, the original version is not serializable because of the persistent lmdb env if you use forkserver/spawn (the getstate and setstate are needed). Also forking the persistent env across workers causes weird behavior/failures with versions of lmdb > 1.6, it hasnt worked for me on multiple systems which is why we pinned it. They added some documentation about this recently: https://lmdb.readthedocs.io/en/latest/#forking-multiprocessing

@christinaflo
Copy link
Copy Markdown
Collaborator Author

Minor comment: the DB preloading logic might be better handled by using vmtouch, which is available in conda‑forge and often present on HPC systems, falling back to reading the whole DB file only when vmtouch is not available. Conceptually, this would only need to be done once per node, since the page cache is shared across processes.

If using vmtouch, one could optionally consider daemon mode and page locking to reduce the risk of eager eviction under memory pressure, although this may or may not be desirable depending on system limits and sharing policies (I assume we typically reserve full nodes?). Overall, I suspect the usefulness of this depends strongly on DB size vs memory size, access locality, and competition for memory.

I just found this which could be vendored. I never used it.

I've never used it before, it isnt currently available on the cluster im using but happy to try it out via conda forge. The biggest db we have is around 10 GB. On one system, i did notice that I had to "rewarm" the cache between epochs so this could be useful to avoid that.

@jandom jandom added the safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing. label Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe-to-test Internal only label used to indicate PRs that are ready for automated CI testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants